Skip to content

refactor(cli): RunE migration, auth dedup, TTY-aware spinner, non-interactive guards - #158

Merged
tim-thacker-nullify merged 3 commits into
mainfrom
refactor/command-hygiene
Jul 31, 2026
Merged

refactor(cli): RunE migration, auth dedup, TTY-aware spinner, non-interactive guards#158
tim-thacker-nullify merged 3 commits into
mainfrom
refactor/command-hygiene

Conversation

@tim-thacker-nullify

@tim-thacker-nullify tim-thacker-nullify commented May 26, 2026

Copy link
Copy Markdown
Member

Claude

What

Command hygiene refactor of the hand-written CLI commands. Behavior — including exit codes and output streams — is preserved; this is a structural/testability pass only.

1. os.ExitRunE

Bespoke commands previously used Run: with os.Exit(...) mid-body, which skips defer logger.Close(ctx) and is untestable. Converted auth (login/logout/status/token/switch/config), fix, status, findings, ci (gate/report), open, whoami, init, mcp, pentest, repos, and update to RunE returning errors.

Error → exit-code mapping is now centralized in one place:

  • cmd/cli/cmd/exitcodes.go defines a typed exitCodeError plus withExitCode / authError / networkError / findingsError constructors and ExitCodeForError.
  • cmd/cli/main.go calls os.Exit(cmd.ExitCodeForError(err)).
  • rootCmd.SilenceUsage = true (no usage dump on runtime errors) and SilenceErrors = false (cobra prints the error once).

Exit codes are unchanged: ExitAuthError (2), ExitNetworkError (3), ExitFindings (1), malformed-host (1). The ci gate command keeps its stdout gate-failure summary and silences cobra's stderr echo so its output stays byte-identical.

The generated api commands and their getAPIClient factory are untouched — resolveHost keeps an os.Exit wrapper delegating to the new error-returning resolveHostE for that path.

2. Auth boilerplate dedup

fix, status, findings, and ci each repeated resolveHost → GetNullifyToken → NewNullifyClient → LoadCredentials → extract QueryParameters. They now route through the existing resolveCommandAuth helper (made error-returning, with a new commandAuthContext.Client()).

3. TTY-aware spinner

internal/output/spinner.go NewSpinner now stays silent when stderr is not a terminal, so it no longer pollutes CI logs with ANSI escape sequences and braille frames.

4. Non-interactive guards

auth login host prompt and the init wizard's domain step now fail fast with a clear error ("not a terminal; pass --host …") when stdin is not a TTY, instead of blocking forever on a read in CI/pipes.

Testing

  • GOWORK=off CGO_ENABLED=0 go build ./...
  • GOWORK=off go vet ./...
  • GOWORK=off go test ./... (all green)
  • golangci-lint run on changed packages (0 issues)
  • Added exitcodes_test.go covering the central exit-code mapping.

🤖 Generated with Claude Code

Final base-integration follow-up

Merged current main in c7856cc03ad and resolved the command-hygiene conflicts while preserving the newer fail-closed CI gate, SARIF reporting, refreshing MCP API client, and dependency-analysis exit codes.

The handwritten scan, sbom, and threat commands added on main now follow the same RunE and centralized error path. The generated-command compatibility wrapper remains intact. The API generator target now runs the full generator package, and generated Go output is normalized deterministically without trailing blank lines.

Validation on the merged head:

  • GOWORK=off go test ./...
  • GOWORK=off CGO_ENABLED=0 go build ./...
  • GOWORK=off go vet ./...
  • golangci-lint run ./... (0 issues)
  • GOWORK=off make generate-api, followed by a second generation pass with no working-tree drift
  • git diff --check

…eractive guards

Command hygiene refactor for the hand-written CLI commands. Behavior
(including exit codes and output streams) is preserved.

- os.Exit -> RunE: convert bespoke command bodies (auth, fix, status,
  findings, ci, open, whoami, init, mcp, pentest, repos, update) to
  return errors instead of calling os.Exit mid-body, so deferred
  logger.Close runs and the commands are testable. Centralize
  error->exit-code mapping in main.go via a typed exitCodeError and
  ExitCodeForError; preserves ExitAuthError(2), ExitNetworkError(3),
  and ExitFindings(1). Set rootCmd.SilenceUsage=true /
  SilenceErrors=false so cobra prints each error once.
- Auth boilerplate dedup: route fix/status/findings/ci through the
  existing resolveCommandAuth helper (now error-returning) instead of
  repeating resolveHost->GetNullifyToken->NewNullifyClient->
  LoadCredentials. Added commandAuthContext.Client().
- TTY-aware spinner: NewSpinner stays silent when stderr is not a
  terminal, avoiding ANSI/braille noise in CI logs.
- Non-interactive guards: auth login host prompt and the init wizard
  domain step now fail fast with a clear error when stdin is not a TTY
  instead of blocking on a read.

The generated `api` commands and their getAPIClient factory are
unchanged (resolveHost retains its os.Exit wrapper for that path).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@tim-thacker-nullify tim-thacker-nullify added the patch Patch version updates (fixes) label May 26, 2026
@tim-thacker-nullify

Copy link
Copy Markdown
Member Author

Claude

Status check (2026-05-28): still relevant. grep "os.Exit" cmd/cli/cmd/*.go returns 40+ hits on main across auth, ci, fix, findings, mcp, root, repos, init, pentest, whoami, status, update, open — none of the RunE migration landed in another PR. internal/output/spinner.go is still TTY-unaware.

Nothing on main supersedes any of it. Recommend rebase + un-draft.

@tim-thacker-nullify

Copy link
Copy Markdown
Member Author

Claude

CODEX COUNCIL VERDICT: APPROVED

Advisory AI review completed against merged head c7856cc03ad.

  • The branch now merges cleanly with current main.
  • No human review comments, formal change requests, or unresolved inline review threads were found.
  • Conflict resolution preserves the newer fail-closed CI gate, SARIF reports, refreshing MCP client, and dependency-analysis exit codes.
  • Handwritten scan, sbom, and threat commands added on main now use RunE, return through centralized exit handling, and preserve deferred logger cleanup.
  • The API generation target invokes the complete generator package, and generated output is deterministic and whitespace-clean.
  • CODEOWNERS lists only the PR author, so this verdict does not substitute for independent human approval.

Validation:

  • GOWORK=off go test ./...
  • GOWORK=off CGO_ENABLED=0 go build ./...
  • GOWORK=off go vet ./...
  • golangci-lint run ./...0 issues
  • GOWORK=off make generate-api, repeated with no working-tree drift
  • git diff --check

GitHub Actions restarted for the merged head. No failing checks are present at the time of this verdict; queued checks still need to complete.

@tim-thacker-nullify
tim-thacker-nullify marked this pull request as ready for review July 29, 2026 02:33
Comment thread cmd/cli/cmd/auth.go Outdated
dnjg
dnjg previously approved these changes Jul 31, 2026
@tim-thacker-nullify
tim-thacker-nullify merged commit 731c73e into main Jul 31, 2026
4 checks passed
@tim-thacker-nullify
tim-thacker-nullify deleted the refactor/command-hygiene branch July 31, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch Patch version updates (fixes)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants